Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Hono] Replace usage of pem truststore by JKS #257

Closed
wants to merge 2 commits into from
Closed

[Hono] Replace usage of pem truststore by JKS #257

wants to merge 2 commits into from

Conversation

b-abel
Copy link
Contributor

@b-abel b-abel commented Jun 7, 2021

As discussed in #237 (comment):
Kafka (in the version currently used by Hono) does not support PEM files. Instead of providing the same certificates in PEM as well as in JKS format to Hono's components, this replaces the usage of the file trusted-certs.pem with the file trustStore.jks. The JKS file requires a password which is added to the configuration as well.

@b-abel
Replace usage of pem truststore by JKS.
325caf4 
Kafka (in the version currently used by Hono) does not support PEM files.
Instead of providing the same certificates in PEM as well as in JKS format
to Hono's components, this replaces the usage of the file `trusted-certs.pem`
with the file `trustStore.jks`. The JKS file requires a password which is
added to the configuration as well.

Signed-off-by: Abel Buechner-Mihaljevic <[email protected]>
@b-abel b-abel requested review from calohmn and ctron as code owners June 7, 2021 12:16
@b-abel
Bump Hono chart version.
c2638ff 
Signed-off-by: Abel Buechner-Mihaljevic <[email protected]>
@b-abel
Copy link
Contributor Author

b-abel commented Jun 7, 2021

What I don't like about this PR: the JKS file requires a password which is now hardcoded in many places in the chart. While it would be possible to reduce the number of occurrences by referencing a property, we will hardly achieve having only a single place where it can be changed.
Are we sure that this change is worth the effort?

@b-abel b-abel mentioned this pull request Jun 7, 2021
Merged
@sophokles73
Copy link
Member

What I don't like about this PR: the JKS file requires a password

Didn't the JKS file already require one? The JKS file has not been introduced by this PR, has it? It had been introduced in #237, hadn't it?

@b-abel
Copy link
Contributor Author

b-abel commented Jun 7, 2021

What I don't like about this PR: the JKS file requires a password

Didn't the JKS file already require one? The JKS file has not been introduced by this PR, has it? It had been introduced in #237, hadn't it?

Sorry, that was misleading. The JKS file has always needed one (even before #237). FMPOV this is a good thing. But the JKS file is now used referenced in the application.yaml files of Hono's components and there the password needs to be provided every time as well. That was not the case with the PEM file because it does not need a password. So, the security is not worse than before, from a usability perspective it is just a stumbling block for users who change the password of the JKS when creating new certificates.

@b-abel
Copy link
Contributor Author

b-abel commented Jun 14, 2021

I would base a new solution on eclipse-hono/hono#2731.

@b-abel b-abel closed this Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calohmn calohmn Awaiting requested review from calohmn calohmn is a code owner

@ctron ctron Awaiting requested review from ctron

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@b-abel @sophokles73